fix(spark): resolve vortex.workerThreads case-insensitively - #9170
fix(spark): resolve vortex.workerThreads case-insensitively#9170jackylee-ch wants to merge 1 commit into
Conversation
4e741ba to
6f53ef2
Compare
| private static final long serialVersionUID = 1L; | ||
|
|
||
| private final ImmutableList<String> dataColumnNames; | ||
| private final ImmutableMap<String, String> formatOptions; |
There was a problem hiding this comment.
I thought we can make this a CaseInsensitiveStringMap, and probably go higher in the call stack and start with CaseInsensitiveStringMap wherever those options originate from
There was a problem hiding this comment.
CaseInsensitiveStringMap isn't Serializable, and formatOptions crosses two serialization boundaries — the VortexPartitionReaderFactory field and the VortexFilePartition record component.
How about a small VortexOptions wrapper, along the lines of Spark's own ParquetOptions? FileSourceOptions is Serializable, holds its CaseInsensitiveMap as transient, and resolves the options it needs into plain fields up front — that gets us case-insensitive lookup and serializability at the same time, and gives the options a single home instead of string literals at each call site.
Happy to do it here, or as a follow-up if you'd rather keep this PR to the one-line fix.
There was a problem hiding this comment.
A read option set as vortex.workerThreads never reached the native runtime. VortexTable.newScanBuilder merges the CaseInsensitiveStringMap Spark hands it into the format options; that map lower-cases its keys, so the option arrives spelled vortex.workerthreads while VortexScanBuilder seeds the camelCase spelling with the default of 4. The case-sensitive lookup always read the default one, silently ignoring the user's value. Wrap the options in a CaseInsensitiveStringMap and read through getInt. Signed-off-by: jackylee <qcsd2011@gmail.com>
6f53ef2 to
02466f9
Compare
2 participants
A read option set as
vortex.workerThreadsnever reached the native runtime.VortexTable.newScanBuildermerges theCaseInsensitiveStringMapSpark hands it into the format options. That map lower-cases its keys in the constructor andentrySet()returns the lower-cased delegate, so the option arrives spelledvortex.workerthreads— whileVortexScanBuilderseeds the camelCase spelling with the default of4. Both keys end up in the map and the case-sensitivegetOrDefaultalways read the default one, silently ignoring the user's value.Also validates the value. It was a bare
Integer.parseInt, so a non-numeric value failed inside the executor task withNumberFormatException: For input string: "eight"— naming neither the option nor the accepted range — and a negative value only failed later in the native runtime.Tests
New
VortexPartitionReaderFactoryTest(JVM-only, no session): default, camelCase key, lower-cased key, zero, whitespace, non-numeric and negative../gradlew :vortex-spark_2.13:test --tests 'dev.vortex.spark.read.VortexPartitionReaderFactoryTest'— 7/7 pass./gradlew :vortex-spark_2.12:test ...— 7/7 passVortexSqlTestandVortexFilterPushdownTest(both Scala versions) — pass, unchangedspotlessCheck(both) andjavadoc— clean